-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support source lookup array results #135
Conversation
|
...ava/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientFactory.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate the approach you've taken; the default implementation without pagination should address the majority of use cases. Even after adding full pagination support, this will remain the default option.
src/main/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClient.java
Outdated
Show resolved
Hide resolved
src/main/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClient.java
Outdated
Show resolved
Hide resolved
src/main/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClient.java
Outdated
Show resolved
Hide resolved
pagination methods. Currently, the connector supports only two simple approaches (`gid.connector.http.source.lookup.result-type`): | ||
|
||
- `single-value` - REST API returns single object. | ||
- `array` - REST API returns array of objects. Pagination is not supported yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if I have a kafka event that matches on the lookup key - is the idea that the kafka event will be enriched with an new field that is an array type. I think a SQL example would be useful to show this.
If there are 2 or more lookup key conditions in the query how will this work with the arrays. I think a SQL example would be useful to show this behaviour also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if I have a kafka event that matches on the lookup key - is the idea that the kafka event will be enriched with an new field that is an array type. I think a SQL example would be useful to show this.
It works differently. For a lookup key there might be multiple matches during lookup. gid.connector.http.source.lookup.result-type=array
indicates that the REST API returns array of objects. As a result of lookup join, multiple rows will be returned. Thanks to the flag the connector knows whether the byte array received should be parsed as single object or an array of objects.
@@ -127,11 +131,13 @@ void shouldQuery200WithParams() { | |||
JavaNetHttpPollingClient pollingClient = setUpPollingClient(getBaseUrl()); | |||
|
|||
// WHEN | |||
RowData result = pollingClient.pull(lookupRowData).orElseThrow(); | |||
Collection<RowData> results = pollingClient.pull(lookupRowData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the existing behaviour for a non-array reult would change to now be an array with one element? If so this would have a migration impact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, from user perspective nothing changes.
|
|
Description
This PR partially addresses #118.
The change allows lookup request to return multiple matching results. Currently, only two approaches are supported (
gid.connector.http.source.lookup.result-type
):single-value
- REST API returns single object.array
- REST API returns array of objects. Pagination is not supported yet.Resolves
HTTP-118
PR Checklist